-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove outdated vscode-ui tests (being replaced with e2e tests) #2584
base: main
Are you sure you want to change the base?
Conversation
I went looking (though haven't literally RTFSed yet), are the cases that were tested here already written with the new infrastructure? |
Not quite. I've created an issue that calls this out: #2586 (and outlines what the existing tests were doing). Since we're not in a rush for the existing PRs blocked by this removal (so that CI will pass), I would feel better if we implemented #2586 ahead of removing these (as long as we can get CI to pass on them). However, it will mean that several of the PRs will be blocked, and that gives me pause. One benefit of prioritizing this would be that the new e2e framework would quickly be used (which would leverage the recent focus that Marcos has on that infrastructure). I believe this PR can wait on #2586 and I'll tag it as blocked. |
Replacement tests are now ready to merge: #2590. Once approved, we can merge this in now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🆗
Intent
Resolves #2585
Type of Change
Approach
Simply removed.
User Impact
No user impact.
Automated Tests
Directions for Reviewers
Successful CI should validate this enough.
Checklist